Make the project structure more generic - #7
Conversation
There was a problem hiding this comment.
Mixing build artifacts and source hurts my brain...
There was a problem hiding this comment.
I need to keep the generated sources out of source control.
I could use node_modules, but that's is a bad idea as explained in the e-mail by @sam-github.
What options are remaining:
- Create a new directory where to put the generated source files, e.g.
html5/generated. - Add more structure to
build, e.g.build/bundlefor browserify output,build/configfor generated sources. - Keep the generated files in
html5/and use a naming convention that allows a simple .gitignore rule, e.g.html5/browser.datasources.jsandhtml5/browser.models.jscan be ignored by rulehtml5/browser.*.js.
Thoughts?
Move the configuration of datasources to datasources.json, add environment-specific configuration files. Modify api/app.ap.js to setup datasources via loopback-boot.
Modify configure.js script to merge all datasource config files and create a single browserify-enabled file. Modify html5/app.html5.js to dynamically register datasources provided in the config file. Modify configuration of the `watch` task to include json files in the change detection list.
Move the definition-releated code from models/*.js to models.json.
Modify api/app.api.js to load modes via `app.boot`.
Modify html5/configure.js to generate a script that will register
all models and statically require() all models/*.js files.
Define client-only models in `html5/client.models.json`.
Rename client datasources:
remote -> db (match shared models)
memory -> local
Move the loop defining all data-sources from hand-written html5/app.html5.js to the code generated by html5/configure.js. This new way is consistent with how models are loaded.
|
Rebased on top of the current master and finished the implementation of model loading. The pull request is mostly done. The code in The only outstanding issue I am aware of is the place for generated models.js and datasources.js. |
bajtos
changed the title
|
The other remaining item: get rid of |
Upgrade loopback to 2.0.0-beta3. Simplify html5/configure.js and remove the hack to convert `"memory"` to `loopback.Memory`, since the built-in connectors are registered in `app.connectors` now.
We have discussed this with @ritch and agreed to land this pull request without that change. The plan is to investigate whether we can make dynamic requires work with browserify by explicitly adding files like
Done. |
|
LGTM |
Make the project structure more generic
There was a problem hiding this comment.
Missed this before... We've gone to quite some lengths to make this pattern not required.
This makes models useful only if an app is available... I'm not entirely against this, but would like to justify the added constraint.
There was a problem hiding this comment.
Missed this before... We've gone to quite some lengths to make this pattern not required.
I know. You don't have to use this pattern as long as you know how to get hold of your app object in different means.
This makes models useful only if an app is available... I'm not entirely against this, but would like to justify the added constraint.
We want to get rid of global static model/datasource registries at some point in the future and provide containers (scopes) in which models are registered. An example of such container is a loopback app object. We have the container semantics partially implemented via app.models and app.datasources.
That means you can't define a new model by calling the global static Model.extend anymore, you need a reference to the container.
So far we recommended require('../app.js'). That works in the old server-side-only project layout, but does not work for isomorphic code - the app object is created in multiple places depending on whether you are running in a browser or on the server.
The pattern module.exports = function(dependencies...) seems like the cleanest solution to me.
On the second thought, since loopback is still maintaining a static global registry, the code can get the model from that registry without requiring the app object:
var Todo = loopback.getModel('Todo');Such approach won't work for boot/* scripts though. We could implement an app singleton accessible via loopback.getApp() to "fix that".
Anyways. IMO the dependency injection is the direction we want to go in the future, thus I'd rather teach people this new approach.
There was a problem hiding this comment.
This makes models useful only if an app is available... I'm not entirely against this, but would like to justify the added constraint.
There is another angle to this: how to group a set of loopback Models into a package that can be shared between projects.
In the current solution, models/ is not a standalone package, it does not export anything. It expects that app.boot() will pick up model.json and models/*.js and do whatever is necessary to setup the models in the app.
To create a package of Modules, one has to use different APIs, i.e. manually call Model.extend or DataModel.extend as was done in the previous revision.
We could come up with a project layout that will keep all models-related files in a single place (a single npm package) and then implement a mechanism for declaratively adding all/selected models from such package to a loopback app. However, I don't think we have enough time for that now, IMO this is closer to "nice to have" than to "must have".
There was a problem hiding this comment.
@ritch I am suggesting to move further discussions to a new GH issue to make sure it does not get lost.
There was a problem hiding this comment.
Yes... this is really important to solve for 2.0.
Remove the dependency on packages, provide sensible static configuration that works for all packages and non-packages alike. Ignore `**/test/**` files. Remove no longer used method `findSrc()`. Note: as of #7, there is no `modules/package.json`, thus the previous nodemon configuration was not picking changes made in `modules/*.js`.
Modify Todo definition and set `properties.created.default` to `Date.now`. This commit is a follow-up for #7, which moved Todo definition to models.json, but did not configured the default value for `created`.
Remove the dependency on packages, provide sensible static configuration that works for all packages and non-packages alike. Ignore `**/test/**` files. Remove no longer used method `findSrc()`. Note: as of #7, there is no `modules/package.json`, thus the previous nodemon configuration was not picking changes made in `modules/*.js`.
2 participants
Refactor project structure to make the boiler plate code more generic. When done, most of the scaffolding code should be moved to loopback-boot and/or loopback-workspace templates.
Task for the next iteration (another pull request):
Todo.getChangeModel()to the app.app.api.jsshould load the configuration fromapp.jsonor keep using the current approach (see TODO in app.api.js)."default": Date.nowviamodels.json/to @ritch please review